Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

EL 6.0 new tests #1217

Merged
merged 10 commits into from
Jan 29, 2024
Merged

EL 6.0 new tests #1217

merged 10 commits into from
Jan 29, 2024

Conversation

markt-asf
Copy link
Contributor

Fixes Issue
New tests for functionality added in EL 6.0

Related Issue(s)
None.

Describe the change

  • ArrayELResiolver tests extended to include new length property
  • Add tests for new RecordELResolver
  • Add tests for new OptionalELResolver
  • Add test for method visibility clarification
    There are also some clean-up commits (no functional changes) to the touched classes to fix various IDE warnings.

Additional context
None.

CC @alwin-joseph @anajosep @arjantijms @cesarhernandezgt @dblevins @m0mus @edbratt @gurunrao @jansupol @jgallimore @kazumura @kwsutter @LanceAndersen @bhatpmk @RohitKumarJain @shighbar @gthoman @brideck @OndroMih @dmatej
@starksm64 @scottmarlow

alwin-joseph
alwin-joseph reviewed Jan 16, 2024
View reviewed changes
el/src/main/java/com/sun/ts/tests/el/api/jakarta_el/arrayelresolver/ELClientIT.java
BareBonesELContext barebonesContext = new BareBonesELContext();
ELContext context = barebonesContext.getELContext();

pass = ResolverTest.testELResolver(context, arrayResolver, colors,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get below exception when run with Glassfish8

java.lang.Exception: java.lang.NumberFormatException: For input string: "length"
        at com.sun.ts.tests.el.api.jakarta_el.arrayelresolver.ELClientIT.arrayELResolverLengthTest(ELClientIT.java:333)
Caused by: java.lang.NumberFormatException: For input string: "length"
        at com.sun.ts.tests.el.api.jakarta_el.arrayelresolver.ELClientIT.arrayELResolverLengthTest(ELClientIT.java:330)

@markt-asf
Copy link
Contributor Author

Glassfish may need an EL API update. I'm working on a 6.0.0-M2 now.

alwin-joseph
alwin-joseph approved these changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@alwin-joseph alwin-joseph left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests except arrayELResolverLengthTest pass with Glassfish8-M1. Will retest when EL 6.0.0-M2 is integrated with glassfish.

@markt-asf
Copy link
Contributor Author

EL 6.0.0-M2 is now available on Maven Central

@alwin-joseph
Copy link
Contributor

EL 6.0.0-M2 is now available on Maven Central

I was trying EL 6.0.0-M2 with Glassfish. arrayELResolverLengthTest passed.
But the test beanELResolverMethodVisibilityTest fails with below exception.


[ERROR] com.sun.ts.tests.el.api.jakarta_el.beanelresolver.ELClientIT.beanELResolverMethodVisibilityTest  Time elapsed: 0.027 s  <<< ERROR!
java.lang.Exception: 
Test FAILEDsetValue() tested successfully on writable property
Invalid value from getValue():
Value expected: 0
Value retrieved: 0
getType() returns int
non-null as expected
isReadOnly() returns false as expected
getCommonPropertyType() returns java.lang.Object

        at com.sun.ts.tests.el.api.jakarta_el.beanelresolver.ELClientIT.beanELResolverMethodVisibilityTest(ELClientIT.java:419)

@markt-asf
Copy link
Contributor Author

I'll look into that. It was passing for me locally with Tomcat.

@alwin-joseph
Copy link
Contributor

I'll look into that. It was passing for me locally with Tomcat.

It turns out the test passed when the java version is 21, fails in JDK 17.

@markt-asf
Copy link
Contributor Author

Thanks. That is a useful data point. I will look at this but if it is a Java 17 / Java 21 issue then that puts it a lot lower down my priority list.

@alwin-joseph
Copy link
Contributor

Thanks. That is a useful data point. I will look at this but if it is a Java 17 / Java 21 issue then that puts it a lot lower down my priority list.

All tests passed against Glassfish with latest commits. Thanks.

@alwin-joseph alwin-joseph merged commit d3c0d07 into jakartaee:tckrefactor Jan 29, 2024
1 of 3 checks passed
@markt-asf markt-asf deleted the el6-new-tests branch February 21, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alwin-joseph alwin-joseph alwin-joseph approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@markt-asf @alwin-joseph